Skip to content

Conversation

@karpob
Copy link
Contributor

@karpob karpob commented Nov 12, 2025

Description

PR associated with ufo pull request for supporting test data for ufo feature/cmatrix_crtm

needed for ufo PR: https://github.com/JCSDA-internal/ufo/pull/3885

@BenjaminRuston BenjaminRuston added coordinate merge Ready for merge but needs to be coordinated with other repos OBS OBS processing, UFO CRTM CRTM labels Nov 12, 2025
@fcvdb fcvdb self-requested a review November 17, 2025 21:47
@fcvdb fcvdb self-assigned this Nov 17, 2025
@fcvdb
Copy link
Collaborator

fcvdb commented Nov 17, 2025

Some of the added files are quite big (74Mb). We try to keep the repo as small as possible and we would prefer files of size under 1Mb. Would it be possible for you to reduce the files size by subsampling the data? Thanks.

@fcvdb fcvdb removed their assignment Nov 17, 2025
@huishao-r
Copy link
Collaborator

huishao-r commented Nov 25, 2025

@mikecooke77 This is the PR with the large size of testing data to be trimmed. Please let us know if you find anything regarding similar tests for RTTOV.

@mikecooke77
Copy link
Collaborator

@mikecooke77 This is the PR with the large size of testing data to be trimmed. Please let us know if you find anything regarding similar tests for RTTOV.

I think there is a similarly large file for RTTOV in https://github.com/JCSDA-internal/ufo-data/tree/feature/cmatrix_crtm/testinput_tier_1/resources/auxillary

e.g. mtgirs_cmatrix_and_bias

I think both of these files should be stored on git lfs not git. Therefore I would suggest adding both to:

https://github.com/JCSDA-internal/ufo-data/blob/feature/cmatrix_crtm/.gitattributes

You can just add the full filename. Then you will want to renormalize the repo e.g.

git add --renormalize .

@BenjaminRuston
Copy link
Collaborator

@mikecooke77 agree with putting the file in git-lfs

@karpob will do this tomorrow if you don't beat us to it. @rajichidamb added you as a reviewer

@karpob
Copy link
Contributor Author

karpob commented Jan 8, 2026

@rajichidamb thanks! I was literally just about to look at this.

@rajichidamb
Copy link
Contributor

@rajichidamb thanks! I was literally just about to look at this.

Please feel free to fix if I missed anything.

@karpob
Copy link
Contributor Author

karpob commented Jan 8, 2026

@rajichidamb thanks! I was literally just about to look at this.

Please feel free to fix if I missed anything.

Looks good to me!

@BenjaminRuston
Copy link
Collaborator

@mikecooke77 we took your advice and put the files in git-lfs can you confirm on your end that all looks correct,,, thx

@BenjaminRuston BenjaminRuston added the needs review Asking others to review - often used for pull requests label Jan 12, 2026
@BenjaminRuston
Copy link
Collaborator

tests are passing with this and associated UFO PR, locally using spack-stack v1.8 and gnu compilers

(venv) benr@jedi-wheels:~/work/JCSDA/JEDI/jedi-localhost-gnu/build/ufo$ ctest -R reconrad
Test project /home/benr/work/JCSDA/JEDI/jedi-localhost-gnu/build/ufo
    Start 836: ufo_test_tier1_test_ufo_iasi_crtmreconrad_linop_check
1/2 Test #836: ufo_test_tier1_test_ufo_iasi_crtmreconrad_linop_check ........   Passed    2.71 sec
    Start 837: ufo_test_tier1_test_ufo_iasi_crtmreconrad_linop_tlad_check
2/2 Test #837: ufo_test_tier1_test_ufo_iasi_crtmreconrad_linop_tlad_check ...   Passed    9.07 sec

100% tests passed, 0 tests failed out of 2

Label Time Summary:
crtm         =  11.78 sec*proc (2 tests)
mpi          =  11.78 sec*proc (2 tests)
operators    =  11.78 sec*proc (2 tests)
script       =  11.78 sec*proc (2 tests)
ufo          =  11.78 sec*proc (2 tests)

Total Test time (real) =  11.86 sec

Copy link
Collaborator

@BenjaminRuston BenjaminRuston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @karpob the addition to git-lfs comes with a monetary impact we should ensure this isn't excessive I'm not sure what that threshold is

@BenjaminRuston
Copy link
Collaborator

@rajichidamb can you check with @eap about the potential impact on our git-lfs quota and if this is substantial enough to change course

@BenjaminRuston BenjaminRuston requested a review from eap January 14, 2026 20:32
@BenjaminRuston BenjaminRuston changed the title add test data for crtm reconstructed radiance operator. add test data for crtm reconstructed radiance operator Jan 14, 2026
Copy link
Contributor

@rajichidamb rajichidamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @karpob

@BenjaminRuston BenjaminRuston merged commit 33e0d22 into develop Jan 15, 2026
@BenjaminRuston BenjaminRuston deleted the feature/cmatrix_crtm branch January 15, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coordinate merge Ready for merge but needs to be coordinated with other repos CRTM CRTM needs review Asking others to review - often used for pull requests OBS OBS processing, UFO

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants